feat(acp): ACP Server with WebSocket transport by chaodu-agent · Pull Request #1260 · openabdev/openab

chaodu-agent · 2026-06-30T03:51:04Z

Summary

Implements Phase 1 MVP of the ACP Server per ADR #1258.

Any ACP-compliant client (Zed, JetBrains, desktop apps, web apps, CLIs) can now connect to OAB via WebSocket and interact with the multi-agent platform using the standard Agent Client Protocol.

Architecture

ACP Client ──WSS GET /acp──> OpenAB
                              │
  initialize ────────────────>│──> capability negotiation
  session/new ───────────────>│──> create session + channel_id
  session/prompt ────────────>│──> GatewayEvent → Dispatcher → Agent
                              │
  <── session/notification ───│<── GatewayReply → AgentMessageChunk
  <── session/notification ───│<── (streaming deltas)
  <── prompt response ────────│<── {stopReason: "endTurn"}

What's implemented (Phase 1)

GET /acp WebSocket upgrade endpoint
Bearer token auth (OPENAB_ACP_AUTH_KEY env var)
initialize — protocol version + capability negotiation
session/new — creates OAB session with internal channel mapping
session/prompt — converts prompt to GatewayEvent, dispatches through existing pipeline
Reply streaming — GatewayReply translated to AgentMessageChunk notifications (delta-based)
session/cancel — placeholder
Reply registry (AcpReplyRegistry) for routing replies back to the correct WebSocket
Feature-gated behind acp feature flag

Files changed

File	Change
`Cargo.toml`	Add `acp` feature flag
`crates/openab-gateway/Cargo.toml`	Add `agent-client-protocol` optional dep + `acp` feature
`crates/openab-gateway/src/adapters/acp_server.rs`	New — ACP server adapter (561 lines)
`crates/openab-gateway/src/adapters/mod.rs`	Register `acp_server` module
`crates/openab-gateway/src/lib.rs`	Add ACP fields to AppState, wire route + reply dispatch

Configuration

export OPENAB_ACP_ENABLED=true
export OPENAB_ACP_AUTH_KEY="your-secret-key"  # optional but recommended

What's next (Phase 2+)

Tool call notifications (ToolCall, ToolCallUpdate)
requestPermission flow for dangerous operations
session/load — resume existing sessions
Multi-session per connection
Agent routing via model parameter
Streamable HTTP transport (POST + SSE)

Testing

# Build with ACP support
cargo build --features acp

# Connect with any WebSocket client
websocat ws://localhost:8080/acp?token=your-key

# Send initialize
{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"v1","clientInfo":{"name":"test","version":"1.0"}}}

Refs: ADR PR #1258
cc @pahud

Add an ACP-compliant server endpoint at GET /acp that accepts WebSocket connections and speaks JSON-RPC 2.0 per the Agent Client Protocol spec. Implements: - WebSocket upgrade with Bearer token auth (OPENAB_ACP_AUTH_KEY) - initialize: capability negotiation - session/new: create OAB session mapped to internal channel - session/prompt: dispatch to agent via GatewayEvent, stream back AgentMessageChunk notifications from GatewayReply - session/cancel: placeholder for Phase 2 Architecture: ACP Client --WS JSON-RPC--> /acp endpoint --> GatewayEvent GatewayReply --> AcpReplyRegistry --> AgentMessageChunk notification --> Client Feature-gated behind 'acp' feature flag. Enable with: OPENAB_ACP_ENABLED=true cargo run --features acp Refs: ADR PR #1258

F1: Use subtle::ConstantTimeEq for auth token comparison (timing attack) F2: Remove duplicate AcpConfig/registry construction in serve() F3: Validate jsonrpc == "2.0" per spec (reject with -32600) F4: Defer reply channel creation to session/prompt (no dead _reply_rx) + Add tracing::debug for reply send failure (race condition visibility)

Critical fixes: - 🔴 Reject concurrent prompts per session (-32001 'Session busy') - 🔴 Abort prompt tasks on disconnect to prevent registry leaks - 🔴 Clean up registry entries after prompt completes (not just on Done) Important fixes: - 🟡 Check event_tx.send() result — return JSON-RPC error if no agent connected - 🟡 Remove unused agent-client-protocol dep (Phase 1 is manual JSON-RPC) - 🟡 Add 'acp' to unified feature list in root Cargo.toml - 🟡 Switch AcpReplyRegistry from tokio::sync::Mutex to std::sync::Mutex (operations are CPU-bound, never held across .await) Also: - Rewrite acp_server.rs for clarity after multiple incremental fixes - Add busy flag per session to track in-flight prompts - Prompt handler now properly cleans up registry on all exit paths

chaodu-agent · 2026-06-30T04:08:06Z

CHANGES REQUESTED ⚠️ — All critical issues from prior rounds resolved; one remaining duplicate-state construction in serve().

What This PR Does

Adds ACP (Agent Client Protocol) server support to OpenAB, enabling any ACP-compliant client (Zed, JetBrains, desktop apps, CLIs) to connect via WebSocket at GET /acp and interact with the multi-agent platform using standard JSON-RPC 2.0.

How It Works

A new acp_server adapter behind an acp feature flag implements JSON-RPC over WebSocket: initialize → session/new → session/prompt. Prompts are converted to GatewayEvent and dispatched through the existing pipeline. Replies stream back as delta-based AgentMessageChunk notifications via an AcpReplyRegistry (std::sync::Mutex). Feature-gated at workspace and crate level. Concurrent prompts on the same session are rejected. All cleanup paths handle registry removal and busy-flag release.

Findings

#	Severity	Finding	Location
1	🟡	Duplicate `AcpConfig::from_env()` in `serve()` — called twice, creates two independent registries	`lib.rs:443-445`
2	🟢	Timing-safe token comparison using `subtle::ConstantTimeEq`	`acp_server.rs:131`
3	🟢	JSON-RPC 2.0 version validation with error code `-32600`	`acp_server.rs:175`
4	🟢	Concurrent prompt rejection with `-32001` busy guard	`acp_server.rs:282`
5	🟢	Prompt task abort on disconnect + full registry cleanup	`acp_server.rs:218`
6	🟢	`event_tx.send()` error handled — returns JSON-RPC error if no agent connected	`acp_server.rs:315`

Finding Details

🟡 F1: Duplicate AcpConfig::from_env() in serve()

In the serve() function, the AppState struct literal calls AcpConfig::from_env() twice:

acp: adapters::acp_server::AcpConfig::from_env(),
acp_reply_registry: adapters::acp_server::AcpConfig::from_env()
    .map(|_| adapters::acp_server::new_reply_registry()),

Issues:

Two separate from_env() calls — the second could theoretically see different env state
Creates a registry even if the first call returned None (they are independent)
AppState::from_env() (line 164-167) already does this correctly with a single extraction

Fix: Extract once before the struct literal, matching the pattern in from_env():

#[cfg(feature = "acp")]
let acp = adapters::acp_server::AcpConfig::from_env();
#[cfg(feature = "acp")]
let acp_reply_registry = acp.as_ref().map(|_| adapters::acp_server::new_reply_registry());

Then reference the variables in the struct literal. This ensures the registry is only created when ACP is enabled and both fields share the same from_env() result.

🟢 F2–F6: All prior findings addressed

Timing-safe auth — subtle::ConstantTimeEq eliminates the timing attack vector
JSON-RPC validation — rejects non-"2.0" requests per spec
Concurrent prompt guard — busy flag prevents dual-dispatch on same session
Disconnect cleanup — aborts in-flight tasks, removes all registry entries
Error propagation — event_tx send failure returns -32603 to client instead of hanging

Baseline Check

PR opened: 2026-06-30
Main already has: ACP client infrastructure (openab-core/src/acp/) for connecting to agent CLIs; multiple gateway adapters (telegram, line, feishu, googlechat, wecom, teams)
Net-new value: ACP server adapter — makes OAB itself an ACP endpoint. Entirely new direction, no overlap with existing adapters.

What's Good (🟢)

All critical and important findings from Rounds 1 and 2 are fixed
Clean feature-flag gating — zero impact on non-ACP builds
Proper streaming delta computation (full_text[sent_len..])
Graceful multi-path cleanup: disconnect, timeout, error, and normal completion
Follows existing adapter patterns (consistent with wecom, teams, etc.)
std::sync::Mutex for CPU-bound registry ops — avoids unnecessary async overhead
Good observability via tracing (info on connect/disconnect/dispatch, warn on failures, debug on race conditions)
agent-client-protocol dependency correctly removed (manual JSON-RPC for Phase 1)
acp added to unified feature list for complete builds

CI Status

✅ check — passed
✅ changes — passed
🔄 Smoke tests — in progress
🔄 build-builder — in progress

chaodu-agent requested a review from thepagent as a code owner June 30, 2026 03:51

This comment has been minimized.

Sign in to view

github-actions Bot added the pending-contributor label Jun 30, 2026

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(acp): ACP Server with WebSocket transport#1260

feat(acp): ACP Server with WebSocket transport#1260
chaodu-agent wants to merge 3 commits into
mainfrom
feat/acp-server

chaodu-agent commented Jun 30, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

chaodu-agent commented Jun 30, 2026

🟡 F1: Duplicate AcpConfig::from_env() in serve()

🟢 F2–F6: All prior findings addressed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

chaodu-agent commented Jun 30, 2026

Summary

Architecture

What's implemented (Phase 1)

Files changed

Configuration

What's next (Phase 2+)

Testing

Uh oh!

This comment has been minimized.

This comment has been minimized.

chaodu-agent commented Jun 30, 2026

What This PR Does

How It Works

Findings

🟡 F1: Duplicate AcpConfig::from_env() in serve()

🟢 F2–F6: All prior findings addressed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant